Skip to content

RAT-570: Create Reporter.Output object#697

Open
Claudenw wants to merge 18 commits into
masterfrom
create-output-object
Open

RAT-570: Create Reporter.Output object#697
Claudenw wants to merge 18 commits into
masterfrom
create-output-object

Conversation

@Claudenw

@Claudenw Claudenw commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

This change requires RAT-568 (now merged)

This change is to defines a Reporter.Output class that contains the configuration that was used for the execution, the XML document that was produced, and the ClaimStatistic that captures the numbers various types of files, licenses, license categories and other statistics from the run.

The Reporter.execute() will create a Reporter.Output and will not automatically format the output as specified in the configuration. Formatting output becomes a second step.

Reporter.Output will have 2 methods to format the output

  • format(ReportConfiguration) - will use the output and stylesheet defined in the configuration to format the output.
  • format(IOSupplier stylesheet, IOSupplier output) - will apply an arbitrary stylesheet against the contained document and send the output to the location specified.

This change will make it easier for the Maven version to generate the document in the Mojo.check class and the final output in the Mojo.report class without regenerating the report.

The Reporter.Output will be immutable and constructed using a builder pattern.

The builder will be able to read the 3 components (Configuration, XML document, and ClaimStatistic) from input streams. Each of those components will be extended to include a Serde implementation to write to, and restore from and IO stream.

@Claudenw Claudenw self-assigned this Jul 6, 2026
@Claudenw Claudenw marked this pull request as draft July 6, 2026 13:14
@Claudenw Claudenw force-pushed the create-output-object branch from 6e82596 to 0fcd7da Compare July 8, 2026 09:50
@Claudenw Claudenw marked this pull request as ready for review July 9, 2026 16:41
@Claudenw Claudenw requested a review from ottlinger July 14, 2026 16:54
@Claudenw

Claudenw commented Jul 14, 2026 via email

Copy link
Copy Markdown
Contributor Author

@Claudenw

Copy link
Copy Markdown
Contributor Author

@ottlinger My plan is to fix the Maven and Ant issues when they get moved to separate projects. There will be lots of cleanup there as the code we are running for Maven is a bit old.

@ottlinger ottlinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except for the naming Serde. I'd prefer a named mapper class. WDYT?

/** Map of counter type to value */
private final ConcurrentHashMap<ClaimStatistic.Counter, IntCounter> counterMap = new ConcurrentHashMap<>();

public Serde serde() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to rename this class as well as Serde is rather generic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class name is ClaimStatistic.Serde. In light of the comment above is this specific enough?

reportables = new ArrayList<>();
}

public Serde serde() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a more specific class name as suggested above.

return sources;
}

// for testing access

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something like Guava's @VisibleForTesting somewhere available in Commons? The more I read these comments, the more I prefer this annotation ;)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add the guava library to pick up the @VisibleForTesting. But I think we should do that as a single PR.

Comment thread apache-rat-core/src/test/java/org/apache/rat/utils/StandardXmlFactoryTests.java Outdated
@sonarqubecloud

Copy link
Copy Markdown


public Serde serde() {
return new Serde();
public SerDes serde() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to change the method from serde() to serDes()?

@ottlinger ottlinger self-requested a review July 16, 2026 11:19

@ottlinger ottlinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@ottlinger

Copy link
Copy Markdown
Contributor

The new SonarCloud issues are related to using the non-deprecated injection annotations. The one concerning the unused field is not really clear to me as the sink is set to null in one method, while remains in the other. I'm not sure if that changes something within the maven-site-plugin-run, therefore would keep the code as it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants